Add logr call depth to ktesting Enabled#434
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: nojnhuh The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
This issue is currently awaiting triage. If klog contributors determine this is a relevant issue, they will accept it by applying the The DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
I think this is a duplicate of #431. Care to review it perhaps, and maybe also other pending PRs in klog? |
|
Yep, I'll take a look there and at other PRs. Thanks! /close |
|
@nojnhuh: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What this PR does / why we need it:
I noticed when invoking
make test-integrationin k/k withKUBE_TEST_VMODULE="dynamicresources=5"that I never saw logs at that level in dynamicresources.go even whenKUBE_TEST_VMODULE="*=5"did show them. Copilot identified a bug here as the cause. I confirmed that by setting vmodule tologr=5and then saw logs from all files at that verbosity.At least in the test, the
info.CallDepthgiven to us by logr is 1, and adding that extra frame goes from logr.go to testinglogger_test.go where the logger is invoked:Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #
Special notes for your reviewer:
I took over the existing
TestCallDepthfor a new test here since that didn't seem to be doing anything interesting.Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.
Release note: